Session-row 'Mark issue done' context menu — close GitHub / transition Jira (#526)#527
Merged
Merged
Conversation
…n Jira (#526) Add a right-click item on a session row that moves the linked issue to its done/closed state in one click, removing the manual round-trip to the provider UI when no PR→issue automation closes it on merge. - New `TaskBackend.closeTask(url:)` — required (not capability-gated; every provider can close). Deliberately distinct from `setTaskStatus(.done)`, which for GitHub only moves a Projects-v2 board column and does NOT close the issue. - GitHub → `gh issue close <url>` - GitLab → `glab issue close <number> --repo <slug>` - Jira → reuse `setTaskStatus(.done)` → transitions to the mapped completed status (#523 `jiraStatusMap`, "Done" fallback) - Corveil → reuse `setTaskStatus(.done)` → `closed` - IssueTracker.markIssueDone(sessionID:) mirrors markInReview: resolves the backend (Jira gets the per-workspace statusMap; others detect host from the URL), closes best-effort (auth / transition-not-allowed / already-closed are logged, non-fatal), then flips the Crow session to .completed on success. - SessionListView: context-menu item shown only when the session has a linked ticket, with a provider-flavored label ("Close Issue" for GitHub/GitLab, "Mark Issue Done" for Jira/Corveil). - Per-backend closeTask tests (GitHub/GitLab/Jira/Corveil). Branch on backend/capability, not `if provider == .github` (ADR 0005). Closes #526 🐦⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude <noreply@anthropic.com> Crow-Session: EFCDC679-23F6-4405-8A81-75E7DCA12833
dgershman
approved these changes
Jun 19, 2026
dgershman
left a comment
dgershman
left a comment
Collaborator
There was a problem hiding this comment.
Code & Security Review
Critical Issues
None.
Security Review
Strengths:
closeTaskvalidates URL shape (ProviderManager.parseTicketURLComponents/JiraKey.parse/CorveilTaskID.parse) before shelling out — no unsanitized URL flows to the CLI as an argv slot.gh issue close <url>/glab issue close <iid> --repo <slug>use positional args (no command interpolation) andshellRunner.run(args:)is argv-based, not shell-string — no injection surface on hostile ticket URLs.- Auth errors (
gh/glab/acli/corveilnot authenticated) are caught and logged at the IssueTracker layer; no stack-crash on missing credentials. gh issue close/glab issue closeare idempotent — re-closing an already-closed issue is a no-op exit-0, so a stale row doesn't fail the user.
Concerns:
- None identified. Provider-scope failures (e.g. PAT missing
reposcope) fall through the genericcatchand log without crashing — the session simply doesn't flip to.completed. Acceptable best-effort behavior.
Code Quality
- Wiring is symmetric with
markInReview:AppStatecallback + loading map,AppDelegateregistration,IssueTracker.markIssueDone,SessionService.deleteSessioncleanup,SessionListViewbutton. Five-touchpoint pattern mirrors the existing path — easy to maintain. - Backend dispatch correctly diverges from
markInReview: Jira goes throughtaskBackend(for: .jira, jira: cfg)to threadstatusMap(#523); other providers go throughtaskBackend(forURL:)so GitLab self-hosted gets the rightGITLAB_HOST(unlikemarkInReview, which capability-gates out before that matters). Right call. closeTaskis correctly NOT capability-gated (ADR 0005 in spirit): every provider has a closed state, and the docstring on the protocol method explains why it's distinct fromsetTaskStatus(.done)(which for GitHub only moves the Projects-v2 column).- Provider-flavored label (
"Close Issue"vs"Mark Issue Done") matches each provider's vocabulary. Nice touch. - Tests cover the new surface well: GitHub (happy / invalid URL / command failure), GitLab (argv +
GITLAB_HOSTenv), Jira (defaultDone+ mappedstatusMap["Done"]), Corveil (closedstatus).swift testinCrowProviderruns 52 tests clean locally.
Consider (Green-level polish, not blocking):
IssueTracker.markIssueDoneupdatesassignedIssues[idx].projectStatus = .doneafter success but leavesstate = "open"on the local row. Same minor staleness asmarkInReview— the next refresh poll corrects it. Updating both inline would shave a couple of seconds of stale UI without changing semantics.markInReviewcatchesProviderError.insufficientScopeand callsreportScopeWarning("project")to surface a UI message;markIssueDonelacks that branch, so agh-side scope failure logs to console but doesn't notify the user. Low priority — the action is best-effort and visible idle state already conveys "no-op".SessionListView.sessionContextMenugates the button onsession.ticketURL != nilbut notsession.provider != nil.markIssueDoneguards onsession.providerand returns silently when missing, so a session with a ticket URL but no provider would render a clickable no-op item. Probably unreachable in practice (provider is derived at ticket-set time), but addingsession.provider != nilto the visibility predicate would be cleaner.markIssueDoneTitle(for:)collapses.noneand.jira/.corveilto"Mark Issue Done"— could be written asdefault: return "Mark Issue Done"for one fewer line. Stylistic.
Summary Table
| Color | Meaning | Verdict effect |
|---|---|---|
| Red | Must fix | Request changes |
| Yellow | Should fix | Request changes |
| Green | Consider | Approve allowed |
Findings: 0 Red, 0 Yellow, 4 Green.
Recommendation: Approve — driven by [0 Red, 0 Yellow, 4 Green] findings. Clean implementation, consistent with existing patterns, well-tested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #526
What
Adds a right-click context-menu item on a session row that moves the linked issue to its done/closed state in one click. On workspaces with no PR→issue automation, a merged PR doesn't auto-close its issue, so it had to be closed by hand in the provider UI — this removes that round-trip.
gh issue close <url>glab issue close <number> --repo <slug>jiraStatusMap, "Done" fallback)closedHow
TaskBackend.closeTask(url:)— a required protocol method (not capability-gated; every provider can close an issue). Deliberately separate fromsetTaskStatus(.done), which for GitHub only runs a Projects-v2 board-column mutation and does not close the issue. Jira/Corveil reuse their existingsetTaskStatus(.done)transition (so Jira honors Settings UI to map Crow ticket states ↔ Jira workflow statuses #523's per-workspacejiraStatusMap); GitHub/GitLab shell out to close.IssueTracker.markIssueDone(sessionID:)mirrorsmarkInReview: resolves the backend (Jira gets the matching workspace'sstatusMap; other providers detect provider + host from the ticket URL), closes best-effort (auth / transition-not-allowed / already-closed are logged and swallowed — no crash), then flips the Crow session to.completedon success so the row reflects the closed issue.SessionListView— context-menu item shown only when the session has a linked ticket, with a provider-flavored label ("Close Issue" for GitHub/GitLab, "Mark Issue Done" for Jira/Corveil). Loading state (isMarkingIssueDone) cleaned up on session delete.Branches on backend/capability, not
if provider == .github(ADR 0005).Decisions
.completed(reusing the existingonCompleteSessionpath) so the row moves to the Completed section — one click does both.Testing
closeTasktests: GitHub (close / invalid-URL / command-failure), GitLab (glab issue closeargv +GITLAB_HOST), Jira (default "Done" + mappedstatusMap["Done"]), Corveil (closed).swift testinPackages/CrowProvider— 100 tests, 0 failures.make app— full macOS app builds clean.🤖 Generated with Claude Code